fix(purchase): surface save error on failed-status execution record by cristim · Pull Request #1237 · LeanerCloud/CUDly

cristim · 2026-06-11T08:08:04Z

Problem

COR-10 (#1184): in executeForAccount (internal/purchase/execution.go), the credential-resolution failure branch discarded the SavePurchaseExecution error (_ =), while the post-purchase save thirty lines later treats a save failure as AUDIT LOSS and returns it. If the DB write failed on the credential-failure path, the per-account "failed" row was silently lost with no log and no error, leaving a gap in the History audit trail.

Fix

Handle the failure-path save exactly like the success path: when the save fails, return an AUDIT LOSS: failed to save execution record ... error joined (via errors.Join) with the original credential-resolution error, so neither failure is masked and the underlying save error stays inspectable via errors.Is.

Test evidence

New regression test TestExecuteForAccount_CredentialFailure_SaveErrorSurfaced replicates the real scenario (credential resolution fails AND the failed-row save fails) and asserts both the AUDIT LOSS error and the credential error surface.
Confirmed it FAILS on pre-fix code (save error silently dropped) and passes with the fix.
go build ./... clean; go test ./internal/purchase/... -count=1: 207 passed; go vet clean.
Per the finding guidance, execution_test.go timing assertions were not touched (TEST-05 / TEST-05: Elapsed-wall-time parallelism assertion in purchase fan-out test is CI-load sensitive #1158 is separate).

Closes #1184

coderabbitai · 2026-06-11T08:08:15Z

Warning

Review limit reached

@cristim, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 43 minutes and 57 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0f57ea17-0850-4b44-8bee-df285c5c4aed

📥 Commits

Reviewing files that changed from the base of the PR and between 451a70f and f0eccb8.

📒 Files selected for processing (2)

internal/purchase/execution.go
internal/purchase/execution_test.go

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/cor-10-fix

_{Comment @coderabbitai help to get the list of available commands.}

cristim · 2026-06-11T08:08:22Z

@coderabbitai review

coderabbitai · 2026-06-11T08:08:28Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

The credential-resolution failure branch in executeForAccount discarded the SavePurchaseExecution error (_ =), while the post-purchase save in the same function treats a save failure as AUDIT LOSS. If the DB write failed on the credential-failure path, the per-account "failed" row was silently lost with no trace, leaving a gap in the History audit trail. Handle the failure-path save exactly like the success path: when the save fails, return an AUDIT LOSS error joined with the original credential-resolution error so neither failure is masked and the save error stays inspectable via errors.Is. Regression test asserts both errors surface; it fails on the previous code (save error silently dropped) and passes with the fix. Closes #1184

cristim · 2026-06-19T21:58:31Z

@coderabbitai review

coderabbitai · 2026-06-19T21:58:37Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…lo:10 The new `if saveErr != nil` audit-loss branch added to executeForAccount on this PR pushed its cyclomatic complexity from 10 to 11, tripping the `gocyclo -over 10` pre-commit hook (and leaving the PR UNSTABLE on three retries). Hoist the credential-failure save into a tiny `persistFailedAccountExecution` helper so executeForAccount drops back to 10 while the audit-loss surfacing behaviour and the new regression test (TestExecuteForAccount_CredentialFailure_SaveErrorSurfaced) remain identical: the helper still does `errors.Join(AUDIT LOSS..., cause)` on save-failure and returns `cause` unchanged on success, so neither the credential error nor the save error can be silently dropped. - gocyclo -over 10 internal/purchase/execution.go: clean (executeForAccount=10). - go vet ./internal/purchase/...: clean. - go test ./internal/purchase/... -count=1: 207 passed.

cristim · 2026-06-26T14:36:13Z

Adversarial review notes

Reviewed this as a money-path silent-failure surface (per-account "failed" row dropping silently leaves a re-approval / re-fire window). Findings:

Fixed in `f0eccb8`

UNSTABLE was a real PR-caused gocyclo trip, not a flake. The new if saveErr != nil audit-loss branch pushed executeForAccount from cyclomatic complexity 10 to 11, tripping pre-commit's gocyclo -over 10 hook on all three retries. Lint / Integration / Security failures on this run match the same suite on origin/main@451a70f7 and are pre-existing (errcheck in cmd/, TestPostgresStore_* integration tests on main is red). Extracted the audit-loss save into persistFailedAccountExecution(ctx, acctExec, account, cause) so the function is back at 10 and the semantics (errors.Join, AUDIT LOSS prefix, errors.Is preservation, cause-unmasked-on-save-success) are identical. Confirmed the new regression test still passes.

Considered, no change needed

Sibling save sites (grep SavePurchaseExecution across internal/purchase/ + internal/api/):
- execution.go:277 post-purchase save: already returns AUDIT LOSS: ... (this PR's reference site).
- manager.go:235 root-row finalize in executeAndFinalize: already wraps with ErrAuditLoss.
- approvals.go:164 ApproveAndExecute ApprovedBy stamp: AUDIT GAP log-only by design - the atomic TransitionExecutionStatus to approved already landed, only attribution is lost (documented).
- manager.go:421 safeFail explanatory-error save: AUDIT GAP log-only by design - TransitionExecutionStatus(approved → failed) already landed, only the human-readable error string is lost (documented).
- handler_purchases.go:692 scheduleApprovedExecution: returns the save error.
- handler_purchases.go:2004 direct-execute audit-fields stamp: AUDIT GAP log-only - pre-flight attribution, not status.
- reaper.go:218 canonical-error annotation: log + bump Errored, status flip already landed (documented).
Reaper interaction. The credential-failure save creates a brand-new row for the per-account fan-out unit. On save failure there is no DB row at all (not "stuck approved"), so the reaper has nothing to re-drive and the allRecsSafeToRedrive idempotency path is not triggered. No SDK call has fired (credential resolution was the failure), so no money has moved. The aggregate root row still gets finalized by executeAndFinalize (manager.go:235).
Save-vs-SDK ordering. The fix is on the credential-failure branch, which runs before processPurchaseRecommendations (and therefore before any provider SDK call). No double-charge surface.
Goroutine/error-return shape. executeForAccount is invoked synchronously by execution.RunForAccountsWithConcurrency, which collects (Value, Err) per account into results. The returned err is consumed by executeMultiAccount and surfaces as multiAccountPartialError or errAllAccountsFailed - no goroutine swallows it.
Test invariant. TestExecuteForAccount_CredentialFailure_SaveErrorSurfaced asserts (a) errors.Is(err, saveErr) (would fail pre-fix - saveErr was discarded via _ =); (b) Contains(err.Error(), "AUDIT LOSS: ...") (would fail pre-fix - no such substring); (c) the credential cause still surfaces. Pinned the bug-replication invariant per feedback_go_ctx_aware_patterns.md.

Out-of-scope follow-ups

finalizePurchaseStatus (handler_purchases.go:1594) mirrors the anti-pattern this PR fixes. When email-send fails, it tries to flip status=failed and on save-failure logs + returns "pending" to the API caller, masking the failed-failed state. Not on the money path (the row is still in pending so it won't fire), but identical shape. Worth a follow-up issue tracking the sweep.

CR re-requested.

cristim · 2026-06-26T14:37:08Z

@coderabbitai review

coderabbitai · 2026-06-26T14:37:57Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

cristim added triaged Item has been triaged priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/eventually No deadline impact/few Limited audience effort/xs Trivial / one-liner type/bug Defect labels Jun 11, 2026

cristim force-pushed the fix/cor-10-fix branch from 8a08afd to 4b68325 Compare June 19, 2026 21:58

cristim mentioned this pull request Jun 26, 2026

fix(purchase): finalizePurchaseStatus silently swallows save-failure (mirrors COR-10) #1290

Open

cristim mentioned this pull request Jun 26, 2026

Silent save-failure after irreversible Execute can bypass MaxPaymentDailyUSD #1310

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(purchase): surface save error on failed-status execution record#1237

fix(purchase): surface save error on failed-status execution record#1237
cristim wants to merge 2 commits into
mainfrom
fix/cor-10-fix

cristim commented Jun 11, 2026

Uh oh!

coderabbitai Bot commented Jun 11, 2026 •

edited

Loading

Review limit reached

Uh oh!

cristim commented Jun 11, 2026

Uh oh!

coderabbitai Bot commented Jun 11, 2026 •

edited

Loading

Uh oh!

cristim commented Jun 19, 2026

Uh oh!

coderabbitai Bot commented Jun 19, 2026 •

edited

Loading

Uh oh!

cristim commented Jun 26, 2026

Uh oh!

cristim commented Jun 26, 2026

Uh oh!

coderabbitai Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

cristim commented Jun 11, 2026

Problem

Fix

Test evidence

Uh oh!

coderabbitai Bot commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

cristim commented Jun 11, 2026

Uh oh!

coderabbitai Bot commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristim commented Jun 19, 2026

Uh oh!

coderabbitai Bot commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristim commented Jun 26, 2026

Adversarial review notes

Fixed in f0eccb8

Considered, no change needed

Out-of-scope follow-ups

Uh oh!

cristim commented Jun 26, 2026

Uh oh!

coderabbitai Bot commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Jun 11, 2026 •

edited

Loading

coderabbitai Bot commented Jun 11, 2026 •

edited

Loading

coderabbitai Bot commented Jun 19, 2026 •

edited

Loading

Fixed in `f0eccb8`

coderabbitai Bot commented Jun 26, 2026 •

edited

Loading